Add Decimal32, Decimal64, Decimal128#100729
Conversation
|
Note regarding the |
|
This is on my radar to take a look at; just noting it might be a bit delayed due to other priorities for the .NET 9 release. CC. @jeffhandley |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 16 out of 16 changed files in this pull request and generated 3 comments.
Comments suppressed due to low confidence (1)
src/libraries/System.Runtime/ref/System.Runtime.cs:1
- These new numeric types expose
ToString(string?, IFormatProvider?)overloads but do not implementIFormattable. As a result, composite formatting / interpolated string format specifiers (e.g.,string.Format(\"{0:N}\", value)or$\"{value:N}\") may ignore the format string and fall back to parameterlessToString(). Consider addingSystem.IFormattabletoDecimal32/Decimal64/Decimal128(and updating the CoreLib implementations accordingly); optionally also implementISpanFormattableto match other numerics.
|
Hi @tannergooding I have resolved all Copilot comments. You can re-review it when you have time ;) |
| 1000000, | ||
| ]; | ||
|
|
||
| public Decimal32(int significand, int exponent) |
There was a problem hiding this comment.
This still has the pending comment that it shouldn't be public as I'm not convinced its the right shape and UX. Please make it and the corresponding constructor on the other types internal
There was a problem hiding this comment.
Thanks, I have made it internal
| uint comb = decimalBits & NaNMask; | ||
| return comb != NaNMask && comb != InfinityMask; |
There was a problem hiding this comment.
this can do (value & signMask) > Infinity to avoid multiple comparisons or extracting small parts
Resolve #81376